-
-
Notifications
You must be signed in to change notification settings - Fork 874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf(laravel): Caching tables and indexes to avoid multiple queries #6744
perf(laravel): Caching tables and indexes to avoid multiple queries #6744
Conversation
…fault cache driver
/** @var array<int, mixed> $indexes */ | ||
$indexes = Cache::flexible('api-platform.indexes.'.$table, [5, 10], function () use ($schema, $table) { | ||
return $schema->getIndexes($table); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting change, note also #6735. We already have a cache layer so that this isn't called more then once per metadata computation in production mode.
This cache is stored in memory ? File ? How do you make sure it's not stale ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@soyuka the storage engine of the cache is configurable from laravel's cache config in the file config/cache.php
. You can use any cache driver (file, database, redis, memcached, etc) and the second argument to the function [5, 10]
basically tells laravel that the cache is only good for 5 seconds and between 5 to 10 seconds period refresh the cached value automatically so it'll only be at max 5 to 10 seconds old.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@soyuka I logged all the queries and saw multiple calls to the information_schema table for the same table's columns and data and thats why I added this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@soyuka in terms of making sure it is not stale:
https://laravel.com/docs/11.x/cache#swr
But as you pointed out, in production this is not called because it cached when calling the optimize
function (or after the 1st request to the endpoint), so this would apply only to development machines, im not quite sure if or why it would be needed there.
@amermchaudhary Are you running into this issue on a production setup or on your development system?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes @amermchaudhary it's running multiple time on a development environment, but if you set debug to false
metadata gets cached. I'm not a huge fan of multi-layer caching as it's hard to debug when it fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh I was checking on my development system. I didn't know that it was being cached on a production system. In that case we should close this PR. It's not necessary!!
Closng then, thanks for the report @amermchaudhary ! |
This PR uses
Cache::flexible
from Laravel to improve performance so that same table data is not requested multiple times and the request happens in the background